upgradability: key rotation of the auditor public key #1026
Conversation
adecaro
force-pushed
the
f-1005
branch
2 times, most recently
from
ed93e7a to
c7ec9db
Compare
| for _, auditorSignature := range ctx.TokenRequest.AuditorSignatures { | ||
| auditor := auditorSignature.Identity | ||
| // check that issuer of this issue action is authorized | ||
| found := false |
There was a problem hiding this comment.
We could do this like this
found := slices.Contains(auditors, auditorSignature.Identity.Equal)
There was a problem hiding this comment.
okay slices.ContainsFunc.
| if err != nil { | ||
| return errors.Wrapf(err, "failed to deserialize auditor's public key") | ||
| } | ||
| _, err = ctx.SignatureProvider.HasBeenSignedBy(auditor, verifier) |
There was a problem hiding this comment.
If I understand correctly, HasBeenSignedBy has to be called in the right order. Would it make sense to check in any order and keep a counter of how many have been checked already?
There was a problem hiding this comment.
checking in any order is expensive because we would need to verify the signatures against multiple public keys.
Anyhow, a driver can use a different implementation of SignatureProvider as you suggest, sure.
| @@ -248,13 +265,36 @@ func (v *Validator[P, T, TA, IA, DS]) verifyTransfer(tr TA, ledger driver.Ledger | |||
| } | |||
There was a problem hiding this comment.
do you mean if c >= 1?
There was a problem hiding this comment.
no, one time is okay.
| Deserializer: v.Deserializer, | ||
| TransferAction: tr, | ||
| TransferAction: action, | ||
| Ledger: ledger, | ||
| SignatureProvider: signatureProvider, | ||
| MetadataCounter: map[MetadataCounterID]int{}, |
There was a problem hiding this comment.
What I don't find very neat here is that sometimes we pass a transfer action in the context, sometimes an issue action and sometimes none. I would pass none of them and then have different validator types:
type TransferValidator interface {
Validate(Context, TransferAction)
}
Similarly for IssueValidator and AuditValidator
There was a problem hiding this comment.
this sounds like a task to add to the #930
| } | ||
|
|
||
| func (r *AuditorSignature) FromProtos(tr *request.AuditorSignature) error { | ||
| if tr.Identity != nil { |
There was a problem hiding this comment.
maybe also if tr == nil if it can happen
There was a problem hiding this comment.
wondering if we need to address this in another PR.
| func newAuditingViewInitiator(tx *Transaction, local bool) *AuditingViewInitiator { | ||
| return &AuditingViewInitiator{tx: tx, local: local} | ||
| func newAuditingViewInitiator(tx *Transaction, local, skipAuditorSignatureVerification bool) *AuditingViewInitiator { | ||
| return &AuditingViewInitiator{tx: tx, local: local, skipAuditorSignatureVerification: skipAuditorSignatureVerification} |
There was a problem hiding this comment.
Should we include a testing feature in this view?
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
Signed-off-by: Angelo De Caro <[email protected]>
No description provided.